Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix pip install and github actions #86

Merged
merged 4 commits into from
Sep 21, 2023
Merged

Conversation

smoors
Copy link
Collaborator

@smoors smoors commented Sep 20, 2023

fixes #82

tested and works for

  • python 3.6 setuptools 39.2.0
  • python 3.10.4 setuptools 68.0.0

i also moved the eessi dir under src as recommended, see https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#src-layout

@boegel
Copy link
Contributor

boegel commented Sep 20, 2023

@smoors I'm not in favor of moving everything under src... Is there a strong need for that?

Also, we need to make sure we catch the problem being fixed here in CI, so we should update the GitHub Actions workflow first to reproduce the problem with an older setuptools version on top of Python 3.6?

@smoors
Copy link
Collaborator Author

smoors commented Sep 20, 2023

@smoors I'm not in favor of moving everything under src... Is there a strong need for that?

Also, we need to make sure we catch the problem being fixed here in CI, so we should update the GitHub Actions workflow first to reproduce the problem with an older setuptools version on top of Python 3.6?

i don't really see the problem with moving it under src, but i have no strong preference so i reverted it back, it actually makes the setup even simpler.

adding the older setuptools to GH actions is coming up, i first wanted to see if the current ones pass.

@smoors smoors changed the title fix pip install for older setuptools fix pip install and github actions Sep 21, 2023
@smoors smoors added the bug fix label Sep 21, 2023
@smoors
Copy link
Collaborator Author

smoors commented Sep 21, 2023

the last commit imports the pip_install GH action from #87,
which correctly failed, and is now successful for all versions.

Copy link
Contributor

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks a lot for figuring this out @smoors!

@boegel boegel merged commit c2e9100 into EESSI:main Sep 21, 2023
9 checks passed
@boegel boegel added this to the 0.1.0 milestone Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

installation with pip doesn't work with older setuptools versions
2 participants